Skip to content

feat(goal-7): common PR#482

Merged
bedaHovorka merged 20 commits intodevelopfrom
copilot/goal-7-simulation-speed-control
May 7, 2026
Merged

feat(goal-7): common PR#482
bedaHovorka merged 20 commits intodevelopfrom
copilot/goal-7-simulation-speed-control

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 20, 2026 •
edited by bedaHovorka
Loading

Goal 7 requires simulation playback speed control (0.1x–100x) without touching simulation semantics. kDisco runs in pure simulation time with no wall-clock awareness, so speed control is implemented externally via a throttling wrapper.

Tests

All tests from subissues

Copilot AI linked an issue Apr 20, 2026 that may be closed by this pull request
Goal 7: Simulation Speed Control (0.1x-100x) #187
Closed
Copilot AI changed the title [WIP] Add simulation speed control from 0.1x to 100x feat(goal-7): add SimulationRunner with wall-clock speed throttling (Phase 1.1) Apr 20, 2026
Copilot AI requested a review from bedaHovorka April 20, 2026 07:34
@bedaHovorka bedaHovorka marked this pull request as ready for review April 20, 2026 07:35
@bedaHovorka bedaHovorka force-pushed the copilot/goal-7-simulation-speed-control branch 2 times, most recently from 7e7d7f3 to 952e435 Compare April 22, 2026 09:11
@bedaHovorka

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view
bedaHovorka

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view
Copilot AI requested a review from bedaHovorka May 5, 2026 17:44
@bedaHovorka bedaHovorka closed this May 5, 2026
@bedaHovorka bedaHovorka force-pushed the copilot/goal-7-simulation-speed-control branch from df44879 to 56a45a9 Compare May 5, 2026 17:48
@bedaHovorka
Phase 1.2: Wire SimulationRunner into GUI lifecycle (simgui mode + St…
0b6f844 
…op button) (#481)

* Phase 1.2: Add simgui mode, startSimulation/stopSimulation, Stop button, runSimGui task

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/2101345e-19c3-4fe0-92db-8e5076a74071

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* Address code review: @volatile on simulationRunner, document SimulationContext lifecycle

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/2101345e-19c3-4fe0-92db-8e5076a74071

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* Apply review comments: SimulationController extraction, EDT guard, status fix, race fix, KDoc, @disabled test

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/0bd8db88-dc42-4287-a172-3059f6d99658

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* Address code review: improve KDoc defaults, setUp comment, race-condition test explanation, @disabled message

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/0bd8db88-dc42-4287-a172-3059f6d99658

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* Address review 4176008234: stale-monitor guard, FrameSimulationLifecycleTest, parseMode(), context.close, title, EDT KDoc

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/b87cd843-e7ee-47ea-acf8-12a67e742866

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* Extract MSG_CONTEXT_CREATION_FAILED constant to fix SonarCloud duplicated-literal warning

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/84bdd606-117a-4af7-8147-a99811e51eca

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* Status string constants, extract launchMonitorThread, shared showContextInGui helper, extend runExampleGui task

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/ab83f170-ea29-4747-87e5-e47d98516bba

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* Fix CI tearDown race + replace status constants with enum SimulationStatus

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/0100745e-dc4e-450c-8a76-1b9727fee83b

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* Apply review thread: EDT guard on setContext, parseMode dispatch, latch assertions, typo fix

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/549eefdd-ecfc-499e-85e7-55631e7ceff3

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
@bedaHovorka bedaHovorka reopened this May 5, 2026
@bedaHovorka bedaHovorka changed the title feat(goal-7): add SimulationRunner with wall-clock speed throttling (Phase 1.1) feat(goal-7): add SimulationRunner with wall-clock speed throttling May 5, 2026
@bedaHovorka bedaHovorka changed the title feat(goal-7): add SimulationRunner with wall-clock speed throttling feat(goal-7): common PR May 5, 2026
@bedaHovorka

This comment was marked as duplicate.

Sign in to view
@bedaHovorka
Fix FrameSimulationLifecycleTest: toDynamic + PropertyChangeEvent in …
58137f6 
…MockSimulationContext

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/59df2aa5-dcf7-43cc-8d9f-e374bf2ec86b

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

This comment was marked as outdated.

Sign in to view
Copilot AI and others added 5 commits May 6, 2026 15:43
@bedaHovorka @claude
Add golden output tests for simulation speed control (#489)
03bf84f 
* Initial plan

* Initial plan

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* feat: Add SimulationSpeedGoldenTest — Phase 4.1 golden output tests for speed control semantics

Adds 7 integration tests in desktop-ui/src/test/.../gui/SimulationSpeedGoldenTest.kt validating:
- All speed multipliers (0.5x, 1x, 2x, 10x, 50x) produce identical simulation semantics
- Event sequences (timestamps + types) identical across all speed multipliers
- RealTimeSynch at 50x does not alter simulation semantics
- Physics invariants: velocity >= 0, acceleration in [-3.0, 4.0] m/s²
- Train front-positions within 1e-6 m tolerance across all speed multipliers
- Stochastic train generation (exp(43), seed=0) is reproducible across runs
- SimulationRunner speed multiplier does not alter simulation output

All 7 tests pass (total suite: 555 tests, 554 passing, 1 skipped).

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/4cf5facd-49b9-4983-aa91-b60b661838d9

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* refactor: Remove unused TIME_TOLERANCE_S constant (code review feedback)

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/0b62868f-e7fe-4953-8be1-b67cbc41bacf

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* fix: Address all 8 code review comments from PR review

- Frame.setContext: add identity guard to prevent closing same context
- MenuBar speed submenu: replace unused Triple with Pair, remove unused KeyEvent import
- SimulationControlPanel.formatSpeedLabel/formatPresetLabel: use Locale.ROOT
- SimulationSpeedGoldenTest.runViaSimulationRunner: fail test on 20s deadline exceeded,
  wait for thread termination after stop()
- SimulationSpeedGoldenTest physics loop: strict parse with requireNotNull instead of
  silent continue on malformed messages
- SimulationSpeedGoldenTest.extractFrontDistances: strict parse with require/requireNotNull
  instead of mapNotNull filtering
- MockSimulationContext.close: delegate to delegate.close() to release Koin scope
- SimulationController.start: clean up stale speed listener before overwriting runner"

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/f2f63bb4-d82e-49c4-95a3-673e94580877

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* fix: correct TRAIN_CONTINUOUS token indices in SimulationSpeedGoldenTest

SimulationEvent.fromContextChangeEvent splits the raw report string
"{time} Train #9 {content}" with limit=3, making se.message equal to
"#9 {acceleration} {velocity} ...".  The physics-invariants loop and
extractFrontDistances were parsing token indices that assumed no leading
train-id fragment, causing requireNotNull to throw on "#9" when the
strict-parse change was applied in the previous code-review commit.

Fix: shift all token indices by +1 and update the size guards accordingly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: Address 4 code review issues from SimulationSpeedGoldenTest PR

- Frame.startSimulation(): wrap controller call in try/catch and guard
  panel assignment with takeIf { it.isRunning() } to prevent dead runner
  from being wired to SimulationControlPanel if start() fails mid-way
- MenuBar.StartSimulationAction: move XML parsing and context creation
  off the EDT into a SwingWorker (extracted as internal loadSimulationContext);
  shows wait cursor during load, handles errors in done() callback
- SimulationController.SIMULATION_POLL_INTERVAL_MS: reduce 500 ms → 100 ms
  so Stop button reflects natural simulation completion within 100 ms
- MenuBar speed menu: add missing 5x and 50x presets to match the
  authoritative SimulationControlPanel.PRESETS list (was 5 items, now 7)

Tests added: pollIntervalIsResponsive, speedSubmenuHas7Items (renamed from 5),
speedSubmenuItemsHaveCorrectLabels (extended), loadSimulationContextWithValidFile,
loadSimulationContextWithMissingFileThrows

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
Co-authored-by: Bedrich Hovorka <bedrich.hovorka@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@bedaHovorka @claude
Phase 4.2: Performance benchmarks for SimulationRunner speed control (#…
5fc11a9 
…490)

* Initial plan

* Phase 4.2: Add SimulationSpeedPerformanceTest with 6 integration benchmarks

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/98f50808-0413-472e-9cc8-f740f97f7302

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* Address review: monotonic timing, wider overhead baseline, log-only ShuntingLoop, clarify EDT test scope

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/e0801e84-5609-4466-af74-4d38c3256aa1

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* Address review: keyboard presets include 0.5x, increment updates desiredSpeed, single write path in applyPreset

- Remap keys 1-5 to 0.5x/1x/2x/5x/10x so 0.5x is keyboard-accessible
- IncrementalSpeedAction reads SimulationController.speed (runner?.speedMultiplier ?: desiredSpeed)
  instead of early-returning when no runner is active; +/- now updates desiredSpeed before start
- applyPreset routes through onSpeedChanged when wired, falls back to direct runner write otherwise
- Add tests: increment-before-start, preset-via-callback, restart-speed-persistence

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
Co-authored-by: Bedrich Hovorka <bedrich.hovorka@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@bedaHovorka @claude
feat: SimulationSpeedIntegrationTest — Phase 4.3 integration tests fo…
64795ae 
…r speed control (#491)

* feat: add SimulationSpeedIntegrationTest (Goal 7 Phase 4.3)

Integration tests for SimulationRunner speed control covering:
- Rapid speed changes (50x every 10ms) without crash
- Pause/resume at 0.1x, 1x, 10x speeds
- 10 consecutive pause/resume cycles
- Stop at 0.1x speed within 5 seconds
- Stop while paused within 5 seconds
- Concurrent speed changes from 4 threads (no data races)
- Stop at 25%, 50%, 75% simulation progress
- EDT speed changes with no race conditions

All 12 tests pass.

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/6777e7f2-5daf-4b00-81c8-4eb635d91ce1

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* fix: update KDoc to list all 12 test scenarios by number

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/6777e7f2-5daf-4b00-81c8-4eb635d91ce1

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* fix: address review feedback on SimulationSpeedIntegrationTest

- Replace `also { it.isDaemon = true; it.start() }` semicolons with
  multiline `also {}` blocks (lines 118, 293)
- Remove flaky wall-clock `< 5_000L` assertions in stopAtSlowSpeed and
  stopWhilePaused tests; rely on waitForStop(5_000) + @timeout
- Add waitForStop(5_000) to tearDown() so a failing test does not leak
  its simulation thread into the next test
- Change throttle(0.01/0.001) to throttle(0.1) where speed=100x was
  producing sleepMs=0 (busy spin); 0.1/100*1000 rounds to 1ms
- Assert allReady.await() result in concurrentSpeedChangesFromMultipleThreads
  so unsynchronized thread starts are caught
- Restructure stopWhilePausedCompletesWithinFiveSeconds to use a
  CountDownLatch (enteringPauseWait) that fires when the sim thread has
  observed isPaused=true and is about to call awaitIfPaused(), making
  the "stop while paused" path deterministic
- Apply same latch pattern to verifyPauseResumeAtSpeed helper; sim loop
  uses awaitIfPaused() directly (public API) rather than timing-based
  Thread.sleep to confirm the sim thread is actually paused

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/d49fc57d-8567-4ff8-a145-9be26fb73581

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* fix: replace Thread.sleep(50) with resumedFromPause latch in verifyPauseResumeAtSpeed

Add a second CountDownLatch that fires after awaitIfPaused() returns so
the test waits for the sim thread to actually exit the pause-wait state
rather than relying on a timing-based sleep.

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/d49fc57d-8567-4ff8-a145-9be26fb73581

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* Add golden output tests for simulation speed control (#489)

* Initial plan

* Initial plan

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* feat: Add SimulationSpeedGoldenTest — Phase 4.1 golden output tests for speed control semantics

Adds 7 integration tests in desktop-ui/src/test/.../gui/SimulationSpeedGoldenTest.kt validating:
- All speed multipliers (0.5x, 1x, 2x, 10x, 50x) produce identical simulation semantics
- Event sequences (timestamps + types) identical across all speed multipliers
- RealTimeSynch at 50x does not alter simulation semantics
- Physics invariants: velocity >= 0, acceleration in [-3.0, 4.0] m/s²
- Train front-positions within 1e-6 m tolerance across all speed multipliers
- Stochastic train generation (exp(43), seed=0) is reproducible across runs
- SimulationRunner speed multiplier does not alter simulation output

All 7 tests pass (total suite: 555 tests, 554 passing, 1 skipped).

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/4cf5facd-49b9-4983-aa91-b60b661838d9

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* refactor: Remove unused TIME_TOLERANCE_S constant (code review feedback)

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/0b62868f-e7fe-4953-8be1-b67cbc41bacf

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* fix: Address all 8 code review comments from PR review

- Frame.setContext: add identity guard to prevent closing same context
- MenuBar speed submenu: replace unused Triple with Pair, remove unused KeyEvent import
- SimulationControlPanel.formatSpeedLabel/formatPresetLabel: use Locale.ROOT
- SimulationSpeedGoldenTest.runViaSimulationRunner: fail test on 20s deadline exceeded,
  wait for thread termination after stop()
- SimulationSpeedGoldenTest physics loop: strict parse with requireNotNull instead of
  silent continue on malformed messages
- SimulationSpeedGoldenTest.extractFrontDistances: strict parse with require/requireNotNull
  instead of mapNotNull filtering
- MockSimulationContext.close: delegate to delegate.close() to release Koin scope
- SimulationController.start: clean up stale speed listener before overwriting runner"

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/f2f63bb4-d82e-49c4-95a3-673e94580877

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* fix: correct TRAIN_CONTINUOUS token indices in SimulationSpeedGoldenTest

SimulationEvent.fromContextChangeEvent splits the raw report string
"{time} Train #9 {content}" with limit=3, making se.message equal to
"#9 {acceleration} {velocity} ...".  The physics-invariants loop and
extractFrontDistances were parsing token indices that assumed no leading
train-id fragment, causing requireNotNull to throw on "#9" when the
strict-parse change was applied in the previous code-review commit.

Fix: shift all token indices by +1 and update the size guards accordingly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: Address 4 code review issues from SimulationSpeedGoldenTest PR

- Frame.startSimulation(): wrap controller call in try/catch and guard
  panel assignment with takeIf { it.isRunning() } to prevent dead runner
  from being wired to SimulationControlPanel if start() fails mid-way
- MenuBar.StartSimulationAction: move XML parsing and context creation
  off the EDT into a SwingWorker (extracted as internal loadSimulationContext);
  shows wait cursor during load, handles errors in done() callback
- SimulationController.SIMULATION_POLL_INTERVAL_MS: reduce 500 ms → 100 ms
  so Stop button reflects natural simulation completion within 100 ms
- MenuBar speed menu: add missing 5x and 50x presets to match the
  authoritative SimulationControlPanel.PRESETS list (was 5 items, now 7)

Tests added: pollIntervalIsResponsive, speedSubmenuHas7Items (renamed from 5),
speedSubmenuItemsHaveCorrectLabels (extended), loadSimulationContextWithValidFile,
loadSimulationContextWithMissingFileThrows

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
Co-authored-by: Bedrich Hovorka <bedrich.hovorka@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* Phase 4.2: Performance benchmarks for SimulationRunner speed control (#490)
* Phase 4.2: Add SimulationSpeedPerformanceTest with 6 integration benchmarks

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/98f50808-0413-472e-9cc8-f740f97f7302

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* Address review: monotonic timing, wider overhead baseline, log-only ShuntingLoop, clarify EDT test scope

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/e0801e84-5609-4466-af74-4d38c3256aa1

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* Address review: keyboard presets include 0.5x, increment updates desiredSpeed, single write path in applyPreset

- Remap keys 1-5 to 0.5x/1x/2x/5x/10x so 0.5x is keyboard-accessible
- IncrementalSpeedAction reads SimulationController.speed (runner?.speedMultiplier ?: desiredSpeed)
  instead of early-returning when no runner is active; +/- now updates desiredSpeed before start
- applyPreset routes through onSpeedChanged when wired, falls back to direct runner write otherwise
- Add tests: increment-before-start, preset-via-callback, restart-speed-persistence

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
Co-authored-by: Bedrich Hovorka <bedrich.hovorka@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* Initial plan

* fix: strengthen multiplePauseResumeCycles with per-cycle liveness check

Replace per-cycle isPaused setter assertions (which only test the
property, not the sim thread) with isRunning() checks so a dead sim
thread is caught at the cycle boundary. Lower Thread.sleep from 50ms
to 10ms — sufficient at 100x speed with 1ms throttle on loaded CI.
Also clarify the verifyStopAfterIterations comment to explain why
the isRunning() check between latch.await and stop() is race-free.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: remove unused isFalse import in SimulationSpeedIntegrationTest

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
Co-authored-by: Bedrich Hovorka <bedrich.hovorka@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@bedaHovorka @claude
Fix RealTimeSynch: remove presvihnuto from hold() to preserve simulat…
a6f73aa 
…ion semantics

When the CI machine was under load at 50x speed, sleepTime occasionally went
negative, setting presvihnuto to a small negative value (≈ -0.000023 s).
That value was then fed into hold(1 + presvihnuto), shifting RealTimeSynch's
simulation-time schedule by ~23 µs and causing SimulationSpeedGoldenTest to
fail with a non-deterministic timestamp mismatch.

The wall-clock overshoot compensation is unnecessary: beginTime is reset at
the start of each interLoopSleep(), so the next iteration's sleepTime
calculation already accounts for drift. Simulation time must always advance
by exactly 1.0 s per RealTimeSynch tick to satisfy the semantic-invariance
contract tested by Phase 4.1 golden-output tests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@bedaHovorka @claude
Fix exampleGui speed buttons: bridge SimulationController -> Shunting…
0beb74d 
…Loop

Speed-control buttons in exampleGui mode had no effect because the
simulation paced via RealTimeSynch, which read speedMultiplier from a
ShuntingLoop constructor val frozen at 1.0 in ExampleRegistry.
SimulationController.setSpeed only updated SimulationRunner.speedMultiplier
(whose throttle() is currently uncalled per its own KDoc), so the chain
button -> controller -> simulation was broken at the last hop.

Introduces SpeedControllable in core/commonMain (KMP-clean, generic — any
main-process can opt in, not ShuntingLoop-specific). ShuntingLoop now
implements it with an @kotlin.concurrent.Volatile var so RealTimeSynch
on the simulation thread reads each iteration's current value.
SimulationController casts the live main process via getMainProcess()
and propagates setSpeed() through both the runner and the controllable,
covering the active pacing path today and the throttle path when wired.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
bedaHovorka
bedaHovorka requested changes May 7, 2026
View reviewed changes
Copy link
Copy Markdown
Owner

@bedaHovorka bedaHovorka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failure is in :desktop-ui:test in the workflow job definition at .github/workflows/gradle-java21.yml, specifically the ./gradlew test step.

The actionable issue from the logs is:

  • 1 test failed out of 563
  • The failing area is in the desktop-ui test suite
  • The logs repeatedly show nearby passing tests such as StatusBar, XMLContextFactoryLenientTest, and ValidationUtils, but the exact failing test name is not present in the retrieved log excerpt
  • The HTML test report path mentioned in the logs is local to the runner and wasn’t available from the repository snapshot:
    desktop-ui/build/reports/tests/test/index.html

There are also Kotlin warnings in:

but these are warnings only, not the cause of the job failure.

Solution

Because the failure is in desktop-ui:test and the visible surrounding tests include UI-oriented tests like StatusBar, the most likely fix is to make the failing test deterministic and independent of environment-specific UI timing, locale, or shared state.

What to change

Focus on the failing test under desktop-ui/src/test/... and apply these fixes if it is UI-related:

1. Remove timing/race sensitivity

If the test depends on Swing event dispatch or asynchronous updates, flush the UI queue before asserting:

import javax.swing.SwingUtilities

private fun flushEdt() {
    SwingUtilities.invokeAndWait {}
}

Use it after UI-triggering actions:

statusBar.updateSpeedIndicator(1.5)
flushEdt()
assertEquals("1.5x", speedLabel.text)

2. Avoid locale-dependent assertions

The logs show an existing test for Locale.ROOT, so ensure formatting code uses it consistently:

val text = String.format(Locale.ROOT, "%.1fx", multiplier)

If production code currently uses default locale, replace it with Locale.ROOT.

3. Reset shared singleton/static state between tests

If StatusBar or XML-related tests share listeners, models, or parser state, add cleanup:

@AfterTest
fun tearDown() {
    producer.unregisterAllListeners()
}

or recreate the component fresh in each test:

@BeforeTest
fun setUp() {
    producer = FakeStatusProducer()
    statusBar = StatusBar(producer)
}

4. Replace fragile exact-string assertions

If the test compares rendered messages, prefer semantic assertions:

assertTrue(speedLabel.text.contains("1.5"))

instead of brittle whitespace or platform-specific full-text comparisons.

Strongest likely code fix

If the failing test is in the StatusBar area, the best production-side hardening is:

private fun formatSpeed(multiplier: Double): String =
    String.format(Locale.ROOT, "%.1fx", multiplier)

and in tests:

@Test
fun `updateSpeedIndicator shows speed text when multiplier is not 1_0x`() {
    statusBar.updateSpeedIndicator(1.5)
    flushEdt()

    assertTrue(speedLabel.isVisible)
    assertEquals("1.5x", speedLabel.text)
}

Also clean up the warnings

Those two “condition is always true” warnings in TrainPathInteractionTest.kt should be simplified, for example:

assertNotNull(path)
assertTrue(path.blocks.isNotEmpty())

instead of:

if (path != null) {
    assertTrue(path.blocks.isNotEmpty())
}

or any equivalent always-true guarded pattern.

Summary

The fix is to update the failing desktop-ui test so it is deterministic on CI: flush Swing/EDT work before assertions, force Locale.ROOT for formatted UI text, and isolate shared state per test. The workflow itself is fine; the failure comes from a single flaky or environment-sensitive unit test in the desktop-ui:test step.

@bedaHovorka
fix: remove flaky startedLatch.await from nullMainProcessNoOp; fix al…
cf9759b 
…ways-true conditions in TrainPathInteractionTest

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/99ffb82e-cf8b-451e-ae49-4f84c54366c8

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 7, 2026

The failure is in :desktop-ui:test in the workflow job definition at [.github/workflows/gradle-java21.yml](https://github.com/bedaHovorka/interlockSim/blob/0beb74dd30190f6ef410451a6fa15078d140ee6e...

Fixed in cf9759b.

The failing test was SimulationControllerSpeedPropagationTest.nullMainProcessNoOp() (confirmed via the uploaded test-results artifact). It timed out because startedLatch.await(5s) blocked until the sim thread called ctx.run(), which took >10s on the busy CI runner.

The fix removes that wait — runner and speedMultiplier are assigned synchronously inside SimulationController.start() before newRunner.start() is called, so the assertions are valid without waiting for the sim thread.

Also cleaned up the always-true train != null conditions in TrainPathInteractionTest.kt (lines 149, 169) as noted in the comment.

Copilot AI requested a review from bedaHovorka May 7, 2026 07:32
Copilot AI and others added 2 commits May 7, 2026 12:04
@bedaHovorka @claude
Correct Goal 7 documentation for kDisco-based speed control (#494)
31ddfc9 
* docs: correct Goal 7 speed control notes
* docs: mark Goal 7 complete

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Bedrich Hovorka <bedrich.hovorka@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@bedaHovorka @claude
Add user documentation for simulation speed control (#493)
7968d67 
* docs: add simulation speed control guide

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/06ab94aa-10ea-4267-b773-aa9b4d7157dc

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* docs: align speed guide with current branch

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/f8072f68-bc35-48ef-94b6-020de0a6b1dc

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* docs: restore stacked speed control documentation

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/943bd48f-c5aa-4ff9-9008-4338e3578772

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* docs: clarify speeds above 50x

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/943bd48f-c5aa-4ff9-9008-4338e3578772

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* docs: polish speed guide wording

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/943bd48f-c5aa-4ff9-9008-4338e3578772

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

* docs: remove internal jargon and improve Limitations/Troubleshooting

- Drop "Goal 7" from the user-facing intro
- Remove Goal 8 integration-point note from keyboard shortcuts
- Reorder Limitations to lead with control ranges before the 100x cap
- Collapse two CPU-bound sentences into one
- Add actionable fix to the Space-key troubleshooting entry

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
Co-authored-by: Bedrich Hovorka <bedrich.hovorka@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
bedaHovorka

This comment was marked as outdated.

Sign in to view

@bedaHovorka
Fix undeterministic tests in desktop UI (#496)
81dd75c 
* fix: eliminate shared latches map and restore startedLatch.await() in nullMainProcessNoOp (issue #495)

Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/57c03945-0a5c-4b57-a850-c1a29f81f146

Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: bedaHovorka <5263405+bedaHovorka@users.noreply.github.com>
@bedaHovorka bedaHovorka requested a review from Copilot May 7, 2026 13:17

This comment was marked as resolved.

Sign in to view

@bedaHovorka
Potential fix for pull request finding
3dae561 
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI requested a review from bedaHovorka May 7, 2026 13:37
@bedaHovorka @claude
fix(gui): route slider speed change exclusively through callback when…
0780a88 
… wired

Mirror applyPreset()'s exclusive-routing pattern in the slider ChangeListener:
when onSpeedChanged is wired, route through it only; fall back to direct write
only when no callback is set. Adds sliderRoutesViaCallbackWhenWired test to
cover the previously-untested production wiring (runner + callback + slider drag).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 7, 2026

Quality Gate Passed Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
81.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@bedaHovorka bedaHovorka merged commit 5a00c67 into develop May 7, 2026
13 checks passed
@bedaHovorka bedaHovorka deleted the copilot/goal-7-simulation-speed-control branch May 7, 2026 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

+1 more reviewer

@bedaHovorka bedaHovorka bedaHovorka approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@bedaHovorka bedaHovorka

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Goal 7: Simulation Speed Control (0.1x-100x)

3 participants

@bedaHovorka